Codecov Report

❌ Patch coverage is 89.42308% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.28%. Comparing base (bced85d) to head (0245dca).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main     #192      +/-   ##
==========================================
+ Coverage   89.03%   89.28%   +0.24%     
==========================================
  Files          46       46              
  Lines        2289     2258      -31     
==========================================
- Hits         2038     2016      -22     
+ Misses        251      242       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Maybe it makes more sense to have self._vector_origin and

@dataclass(frozen=True)
class VectorOrigin:
    name: str
    vector: weakref.ReferenceType[SubControllerVector]

Then, we can dereference the parent vector if we want to do something with knowledge of what controllers are in a group, but also just pass the name to pvi since that is all we need for grouping. I'm unsure of any use cases the vector class has other than pvi grouping though, so I'm not sure is this adds any value.

@shihab-dls I think the trouble of having to keep track of what vector a sub controller is part of would be avoided if SubControllerVector was a subclass of Controller that gets registered directly with its parent, rather it just being a container and registering its children.

Then with a controller hierarchy like

- Odin
 - FP
  - 1
   - HDF
    - FileName 

the path would be ["fp", 1, "hdf"] where integers in the path are treated specially:

• For PVs, it does Prefix:FP:1:HDF:FileName
• For the pvi structure it does {"fp": "fp1": {"hdf": {...}}} (because groups are not allowed to start with a number)
• For GUI groups we could potentially allow names to start with numbers, or just do the same as pvi structure. Probably the latter as otherwise the subscreen for each fp instance would just have "1" as a title...

If this works as I would like, the FP controller vector itself could have attributes and commands. For example here FrameProcessorAdapterController would be a vector with FrameProcessorController elements.

There may be complications with as this, but that is how it looks in my head. Does that make some sense? Happy to pair on this and figure out the details.

@shihab-dls I think the trouble of having to keep track of what vector a sub controller is part of would be avoided if SubControllerVector was a subclass of Controller that gets registered directly with its parent, rather it just being a container and registering its children.

Then with a controller hierarchy like

- Odin
 - FP
  - 1
   - HDF
    - FileName 

the path would be ["fp", 1, "hdf"] where integers in the path are treated specially:

* For PVs, it does `Prefix:FP:1:HDF:FileName`

* For the pvi structure it does `{"fp": "fp1": {"hdf": {...}}}` (because groups are not allowed to start with a number)

* For GUI groups we could potentially allow names to start with numbers, or just do the same as pvi structure. Probably the latter as otherwise the subscreen for each fp instance would just have "1" as a title...

If this works as I would like, the FP controller vector itself could have attributes and commands. For example here FrameProcessorAdapterController would be a vector with FrameProcessorController elements.

There may be complications with as this, but that is how it looks in my head. Does that make some sense? Happy to pair on this and figure out the details.

I like this idea (adopting more from ophyds DeviceVector); I had initially started with this line of thinking, but was trying to think of a way to group controllers without relying on their names, given I think we lose information about what controller is or is not a vector by the time we get to structuring the pvi tree. Starting at "A SubControllerVector is a Controller" is probably a more valuable foundation though, so I'll explore some approaches (and probably ask for your input tomorrow :) )

@shihab-dls I think the trouble of having to keep track of what vector a sub controller is part of would be avoided if SubControllerVector was a subclass of Controller that gets registered directly with its parent, rather it just being a container and registering its children.
Then with a controller hierarchy like

- Odin
 - FP
  - 1
   - HDF
    - FileName 

the path would be ["fp", 1, "hdf"] where integers in the path are treated specially:

* For PVs, it does `Prefix:FP:1:HDF:FileName`

* For the pvi structure it does `{"fp": "fp1": {"hdf": {...}}}` (because groups are not allowed to start with a number)

* For GUI groups we could potentially allow names to start with numbers, or just do the same as pvi structure. Probably the latter as otherwise the subscreen for each fp instance would just have "1" as a title...

If this works as I would like, the FP controller vector itself could have attributes and commands. For example here FrameProcessorAdapterController would be a vector with FrameProcessorController elements.
There may be complications with as this, but that is how it looks in my head. Does that make some sense? Happy to pair on this and figure out the details.

I like this idea (adopting more from ophyds DeviceVector); I had initially started with this line of thinking, but was trying to think of a way to group controllers without relying on their names, given I think we lose information about what controller is or is not a vector by the time we get to structuring the pvi tree. Starting at "A SubControllerVector is a Controller" is probably a more valuable foundation though, so I'll explore some approaches (and probably ask for your input tomorrow :) )

At this point, pvi trees for CA and P4P are constructed such that a parent controller will have a vector entry as:

"vector": {"d": {prefix}:Vector:PVI}

within which, the vector in p4p will have

"v0": {"d": {prefix}:Vector:0:PVI}",
"v1: {"d": {prefix}:Vector:1:PVI}"

whereas in ca it will have

"vector0": {"d": {prefix}:Vector:0:PVI}",
"vector1: {"d": {prefix}:Vector:1:PVI}"

alongside any vector attributes. I'm unaware if there is a reason we don't want to do vector0:... in p4p as well?

P.S. I've also just tested this in ophyd-async and a ControllerVector will map onto a DeviceVector

Unsure about codecov currently, so will request a review to make sure the pvi structure and PV formatting is what we expect

I think the thing that needs to drive this design of this is what we can represent in the PVI tree. I don't think we can have a node in the tree with both named controllers and indexed controllers.

Perhaps add_sub_controller should still only take str and adding indexed controllers to ControllerVector can only be done with __setitem__? We should also disable adding named controllers to ControllerVector and disable Controller from registering controllers with just numbers as the name.

I am wondering if with these restrictions we can revert the controller path being str | int everywhere and instead have the ControllerVector validate the int and then convert it straight to a str. Here and here we are relying on path elements being int to know if it is a vector, but we could just use isdigit instead.

Thoughts?

I think that's reasonable! I've just implemented this. In order to restrict usage of add_sub_controller in ControllerVector and restrict the type of name passed to add_sub_controller in Controller, I've had to make ControllerVector of type BaseController instead of Controller. An example PVA structure output of these changes is:

P4P_TEST_DEVICE:PVI structure 
    alarm_t alarm 
        int severity 0
        int status 0
        string message 
    time_t timeStamp 2025-11-05 15:18:19.649  
        long secondsPastEpoch 1762355899
        int nanoseconds 649368047
    structure display
        string description some controller
    structure value
        structure a
            string rw P4P_TEST_DEVICE:A
        structure b
            string w P4P_TEST_DEVICE:B
        structure table
            string rw P4P_TEST_DEVICE:Table
        structure child_vector
            string d P4P_TEST_DEVICE:ChildVector:PVI

where P4P_TEST_DEVICE:ChildVector:PVI links to:

P4P_TEST_DEVICE:ChildVector:PVI structure 
    alarm_t alarm 
        int severity 0
        int status 0
        string message 
    time_t timeStamp 2025-11-05 15:18:19.649  
        long secondsPastEpoch 1762355899
        int nanoseconds 649484634
    structure display
        string description some child vector
    structure value
        structure vector_attribute
            string r P4P_TEST_DEVICE:ChildVector:VectorAttribute
        structure __1
            string d P4P_TEST_DEVICE:ChildVector:1:PVI
        structure __2
            string d P4P_TEST_DEVICE:ChildVector:2:PVI

where one of the vector children has:

P4P_TEST_DEVICE:ChildVector:1:PVI structure 
    alarm_t alarm 
        int severity 0
        int status 0
        string message 
    time_t timeStamp 2025-11-05 15:18:19.650  
        long secondsPastEpoch 1762355899
        int nanoseconds 649633169
    structure display
        string description some sub controller
    structure value
        structure c
            string w P4P_TEST_DEVICE:ChildVector:1:C
        structure e
            string r P4P_TEST_DEVICE:ChildVector:1:E
        structure f
            string rw P4P_TEST_DEVICE:ChildVector:1:F
        structure g
            string rw P4P_TEST_DEVICE:ChildVector:1:G
        structure h
            string rw P4P_TEST_DEVICE:ChildVector:1:H
        structure j
            string r P4P_TEST_DEVICE:ChildVector:1:J
        structure d
            string x P4P_TEST_DEVICE:ChildVector:1:D
        structure i
            string x P4P_TEST_DEVICE:ChildVector:1:I

Currently, in ophyd-async, this maps to:

p4p_test_device.children() ==
[('a', <ophyd_async.core._signal.SignalRW object at 0x7f5675febc50>), 
('b', <ophyd_async.core._signal.SignalW object at 0x7f5675ebdd10>), 
('table', <ophyd_async.core._signal.SignalRW object at 0x7f5675222d10>), 
('child_vector', <ophyd_async.core._device.Device object at 0x7f5675222f50>)]

So now I'll need to amend ophyd-async to infer a DeviceVector based on the presence of __# children